persist cookies in remote MCP client for sticky sessions#2902
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR correctly implements cookie jar persistence for the remote MCP HTTP client, enabling sticky sessions. The implementation is clean and the new test coverage is solid.
Summary of findings:
- 1 low-severity / informational note (see inline comment)
aheritier
added
area/mcp
…sions Signed-off-by: maxcleme <maxime.clement@docker.com>
maxcleme
force-pushed
the
co-mcp-remote-cookie-jar
branch
from
a84827b to
29c042f
Compare
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One medium finding in the changed code: the new cookie jar is created inside createHTTPClient(), which is called on every Initialize() invocation. Cookies are correctly persisted within a single session (the PR's stated fix), but they are discarded on reconnection, so the sticky-session guarantee does not survive a reconnect cycle.
| return &http.Client{Transport: oauthT}, oauthT | ||
| // Persist cookies across requests | ||
| // So sticky sessions work if implemented by the server (e.g. in a multiple replica setup) | ||
| jar, err := cookiejar.New(nil) |
There was a problem hiding this comment.
[MEDIUM] Sticky-session cookie jar is ephemeral: cookies are lost on reconnection
createHTTPClient() creates a fresh cookiejar on every call, and Initialize() calls createHTTPClient() each time it runs. This means cookies set by the server during one session are discarded when Initialize is called again (e.g. on a disconnect/reconnect).
Within a single session (first request → subsequent requests on the same http.Client) cookies are correctly forwarded, which is the scenario covered by the new test. However if the MCP client reconnects to the same replicated backend, the new client presents an empty jar and the ingress may route it to a different replica — the exact failure mode this PR aims to fix.
To persist cookies across reconnections, the jar should be allocated once (e.g. lazily on the remoteMCPClient struct, or in newRemoteClient) and reused across Initialize calls rather than recreated each time.
aheritier
added
kind/fix
5 participants
Persist session cookies for remote MCP clients
This PR fixes a connectivity issue that surfaces when cagent talks to a remote MCP server fronted by a replicated.
Symptom
After a successful
initialize, the next request (notifications/initialized/ first tool call) intermittently failedbecause it landed on a different ingress replica that had no session state.
Root cause
The ingress provides session stickiness by setting a cookie on the first response. The
*http.Clientbuilt inpkg/tools/mcp/remote.go::createHTTPClienthad nohttp.CookieJar, so the cookie was dropped and subsequent requests were freeto be routed to any replica.
Change
pkg/tools/mcp/remote.go—createHTTPClientnow creates a per-clientcookiejar.New(nil)and assigns it tohttp.Client.Jar. The jar is naturally scoped per remote MCP server (one client perremoteMCPClientper toolset), so cookiescannot leak across toolsets. Servers that do not use cookies are unaffected.
Tests
pkg/tools/mcp/remote_test.go— newTestCreateHTTPClient_PersistsCookiesusinghttptest.Server: the first requestmust arrive without
mcp_sessionand gets aSet-Cookie: mcp_session=abc123; subsequent requests must echo the cookie back.